Skip to content

Conversation

@cavusmustafa
Copy link
Contributor

@cavusmustafa cavusmustafa commented Nov 6, 2025

Summary

  • OpenVINO build script updates
    • Removed "--minimal" argument for python build to include dependencies for examples.
    • By default, the script does not build llm dependencies in cpp runtime build. We added additional argument to include llm dependencies.
  • Updated cmake files and requirements files accordingly.

Test plan

This PR doesn't include new functionality which requires testing.

cc: @mergennachin @cbilgin @digantdesai @ynimmaga @suryasidd @daniil-lyakhov @anzr299

@pytorch-bot
Copy link

pytorch-bot bot commented Nov 6, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/15650

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure, 4 Unrelated Failures

As of commit 32bda53 with merge base 054b15d (image):

NEW FAILURE - The following job has failed:

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 6, 2025
@github-actions
Copy link

github-actions bot commented Nov 6, 2025

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates the OpenVINO backend build infrastructure to improve flexibility and fix configuration issues. The changes remove the --minimal flag from Python builds to include example dependencies, introduce a new --cpp_runtime_llm flag for building with LLM support, and update documentation and configuration files accordingly.

  • Refactored build script to use conditional LLM dependency inclusion via --cpp_runtime_llm flag
  • Updated CMake configuration to use arrays for better argument handling and removed inline OpenVINO backend build in favor of find_package
  • Updated documentation and requirements files to reflect new build options and fix parameter references

Reviewed Changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
backends/openvino/scripts/openvino_build.sh Refactored build script with new --cpp_runtime_llm flag, removed build_llama_runner function, and restructured CMake argument handling
backends/openvino/README.md Updated build documentation to reference new --cpp_runtime_llm flag instead of --llama_runner
backends/openvino/requirements.txt Removed transformers dependency from requirements
examples/openvino/README.md Added stable_diffusion directory structure to documentation
examples/openvino/llama/README.md Updated instructions to use new --cpp_runtime_llm flag and provide explicit CMake commands for building llama runner
examples/openvino/stable_diffusion/README.md Fixed line numbering formatting
examples/openvino/stable_diffusion/requirements.txt Fixed line numbering formatting
examples/models/yolo12/README.md Updated documentation to clarify pip install strategy and fix --video_input to --video_path parameter
examples/models/yolo12/requirements.txt Updated ultralytics version and added opencv-python dependency
examples/models/yolo12/CMakeLists.txt Changed default OpenVINO backend option to OFF, updated XNNPACK library name, and simplified OpenVINO integration to use find_package

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

)

if [[ "$llm_enabled" -eq 1 ]]; then
CMAKE_ARGS+=("-DEXECUTORCH_BUILD_EXTENSION_LLM=ON -DEXECUTORCH_BUILD_EXTENSION_LLM_RUNNER=ON")
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CMAKE_ARGS array elements are incorrectly combined into a single string. The two options -DEXECUTORCH_BUILD_EXTENSION_LLM=ON and -DEXECUTORCH_BUILD_EXTENSION_LLM_RUNNER=ON should be added as separate array elements, not as a single string.

Change:

CMAKE_ARGS+=("-DEXECUTORCH_BUILD_EXTENSION_LLM=ON -DEXECUTORCH_BUILD_EXTENSION_LLM_RUNNER=ON")

To:

CMAKE_ARGS+=("-DEXECUTORCH_BUILD_EXTENSION_LLM=ON" "-DEXECUTORCH_BUILD_EXTENSION_LLM_RUNNER=ON")
Suggested change
CMAKE_ARGS+=("-DEXECUTORCH_BUILD_EXTENSION_LLM=ON -DEXECUTORCH_BUILD_EXTENSION_LLM_RUNNER=ON")
CMAKE_ARGS+=("-DEXECUTORCH_BUILD_EXTENSION_LLM=ON" "-DEXECUTORCH_BUILD_EXTENSION_LLM_RUNNER=ON")

Copilot uses AI. Check for mistakes.
)

if [[ "$llm_enabled" -eq 1 ]]; then
CMAKE_ARGS+=("-DEXECUTORCH_BUILD_EXTENSION_LLM=ON -DEXECUTORCH_BUILD_EXTENSION_LLM_RUNNER=ON")
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line uses a tab character for indentation instead of spaces, which is inconsistent with the rest of the file. Replace the tab with spaces to maintain consistent indentation.

Suggested change
CMAKE_ARGS+=("-DEXECUTORCH_BUILD_EXTENSION_LLM=ON -DEXECUTORCH_BUILD_EXTENSION_LLM_RUNNER=ON")
CMAKE_ARGS+=("-DEXECUTORCH_BUILD_EXTENSION_LLM=ON -DEXECUTORCH_BUILD_EXTENSION_LLM_RUNNER=ON")

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants